Add pre-commit for better linting #25

bmchalenv · 2026-01-14T07:15:39Z

Adds a pre-commit config that uses uncrustify and flake8 just like ament. ament isn't used directly since I wanted formatting to occur outside of a ROS environment when making small fixes. Also has other tools like whitespace, newline, copyright, yaml checks, code spelling, shell script formatting, and commit sign off checking.

Useful for fixing most issues with linting before it is pushed. Automatically reformats your commits, fixes copyright year issues, etc.

To run:

sudo apt install uncrustify pipx
pipx install pre-commit
pre-commit install --hook-type commit-msg --hook-type pre-commit
pre-commit run --all-files  # will also run on every commit now

NOTE: the sign-off check does not run in CI, only locally because it affects the commit message which is unchangeable at that point

greptile-apps · 2026-01-14T07:18:44Z

Greptile Overview

Greptile Summary

This PR introduces a comprehensive pre-commit configuration to automate code quality checks. The changes include adding pre-commit hooks for linting (flake8, uncrustify), formatting (autopep8, shfmt), copyright validation, YAML validation, whitespace cleanup, and spell checking. The PR also integrates these checks into the GitHub Actions CI workflow and documents the setup process in Contributing.md.

Key Changes:

Added .pre-commit-config.yaml with 10+ hooks for code quality enforcement
Integrated pre-commit checks into CI workflow (.github/workflows/ros-tests.yml)
Updated Contributing.md with installation and usage instructions
Applied automated formatting across the entire codebase (copyright year updates, trailing newlines, shell script indentation)
All previous syntax issues mentioned in review threads have been resolved

Impact:

Automated code quality enforcement before commits reach CI
Consistent code formatting across Python, C++, and shell scripts
Reduced manual review burden for style and formatting issues
Better developer experience with automatic fixes for common issues

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The PR adds tooling infrastructure without modifying core functionality. All formatting changes are automated and reversible. The pre-commit configuration is well-structured, uses standard tools (flake8, autopep8, uncrustify), and properly excludes build artifacts. CI integration ensures checks run on all PRs. Previous syntax issues have been resolved.
No files require special attention

Important Files Changed

Filename	Overview
.pre-commit-config.yaml	Adds comprehensive pre-commit configuration with linting, formatting, copyright checks, and code quality tools
.github/workflows/ros-tests.yml	Adds pre-commit job to CI workflow with proper dependency installation
Contributing.md	Documents pre-commit setup instructions and uninstall command
greenwave_monitor_interfaces/package.xml	Adds copyright header (properly formatted with XML comment syntax, not duplicate declaration)
scripts/docker-test.sh	Reformats shell script with consistent indentation (tabs instead of spaces), updates copyright year
scripts/build_debian_packages.sh	Reformats shell script with consistent indentation, updates copyright year

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Git as Git Commit
    participant PreCommit as Pre-commit Framework
    participant Hooks as Pre-commit Hooks
    participant CI as GitHub Actions

    Dev->>Git: git commit -s -m "message"
    Git->>PreCommit: Trigger pre-commit hooks
    
    PreCommit->>Hooks: check-yaml
    Hooks-->>PreCommit: ✓ YAML files valid
    
    PreCommit->>Hooks: end-of-file-fixer
    Hooks-->>PreCommit: ✓ Fixed missing newlines
    
    PreCommit->>Hooks: trailing-whitespace
    Hooks-->>PreCommit: ✓ Removed trailing whitespace
    
    PreCommit->>Hooks: autopep8 (Python formatting)
    Hooks-->>PreCommit: ✓ Python code formatted
    
    PreCommit->>Hooks: flake8 (Python linting)
    Hooks-->>PreCommit: ✓ Python code passes linting
    
    PreCommit->>Hooks: shfmt (Shell formatting)
    Hooks-->>PreCommit: ✓ Shell scripts formatted
    
    PreCommit->>Hooks: copyright-required
    Hooks-->>PreCommit: ✓ Copyright headers present
    
    PreCommit->>Hooks: uncrustify (C/C++ formatting)
    Hooks-->>PreCommit: ✓ C/C++ code formatted
    
    PreCommit->>Hooks: codespell (Spelling check)
    Hooks-->>PreCommit: ✓ No spelling errors
    
    Git->>PreCommit: Trigger commit-msg hook
    PreCommit->>Hooks: signed-off-by check
    Hooks-->>PreCommit: ✓ Commit is signed off
    
    PreCommit-->>Git: All checks passed
    Git-->>Dev: Commit created
    
    Dev->>CI: Push to GitHub
    CI->>CI: Checkout code
    CI->>CI: Install uncrustify + dependencies
    CI->>PreCommit: Run pre-commit on all files
    PreCommit->>Hooks: Execute all hooks
    Hooks-->>PreCommit: ✓ All checks passed
    PreCommit-->>CI: Success
    CI-->>Dev: ✓ CI passed

greptile-apps

_{23 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greenwave_monitor_interfaces/package.xml

greptile-apps

_{23 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greenwave_monitor_interfaces/package.xml

greptile-apps

_{23 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T16:53:43Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+        should_redraw = (
+            key != -
+            1) or (
+            current_time -
+            last_redraw >= redraw_interval)


syntax: Autopep8 broke this expression by splitting -1 across lines. key != - on one line and 1) on the next is incorrect formatting.

Suggested change

should_redraw = (

key != -

1) or (

current_time -

last_redraw >= redraw_interval)

should_redraw = (key != -1) or (current_time - last_redraw >= redraw_interval)

greptile-apps · 2026-01-14T16:53:44Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+        header = (
+            f'{
+                "Topic Name":<{MAX_NAME_WIDTH}} {
+                "Status":<{STATUS_WIDTH}} ' f'{
+                "Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} ' f'{
+                    "Latency (ms)":<{REALTIME_DELAY_WIDTH}} {
+                        "Expected Hz":<12}')


syntax: F-string formatting is broken. Line breaks inside the f-string placeholders create invalid syntax.

Suggested change

header = (

f'{

"Topic Name":<{MAX_NAME_WIDTH}} {

"Status":<{STATUS_WIDTH}} ' f'{

"Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} ' f'{

"Latency (ms)":<{REALTIME_DELAY_WIDTH}} {

"Expected Hz":<12}')

header = (f'{"Topic Name":<{MAX_NAME_WIDTH}} {"Status":<{STATUS_WIDTH}} '

f'{"Pub Rate (Hz)":<{FRAME_RATE_WIDTH}} '

f'{"Latency (ms)":<{REALTIME_DELAY_WIDTH}} {"Expected Hz":<12}')

greptile-apps · 2026-01-14T16:53:45Z

greenwave_monitor/greenwave_monitor/ncurses_frontend.py

+            topic_set = {
+                topic_name for topic_name,
+                topic_type in topic_names_and_types}


syntax: Set comprehension incorrectly split. The comma between topic_name and topic_type should be on the same line.

Suggested change

topic_set = {

topic_name for topic_name,

topic_type in topic_names_and_types}

topic_set = {topic_name for topic_name, topic_type in topic_names_and_types}

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-01-14T16:53:46Z

.pre-commit-config.yaml

+    - id: autopep8
+      args: [-i, -a, -a]


style: The -a -a flags make autopep8 very aggressive, causing the broken formatting in ncurses_frontend.py. Consider using just -i or single -a to avoid overly aggressive line breaking.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T01:01:14Z

.github/workflows/ros-tests.yml

+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git
+        pipx install pre-commit
+        pipx ensurepath
+        source ~/.bashrc
+        pre-commit run --all-files --show-diff-on-failure
+      shell: bash
+      working-directory: greenwave_monitor
+      env:


Pre-commit runs in wrong directory. Code is checked out to greenwave_monitor/ subdirectory (line 56) but this step doesn't cd into it before running pre-commit.

Suggested change

- name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)

run: |

apt update && apt install -y pipx git

pipx install pre-commit

pipx ensurepath

source ~/.bashrc

pre-commit run --all-files --show-diff-on-failure

shell: bash

working-directory: greenwave_monitor

env:

- name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)

run: |

cd greenwave_monitor

apt update && apt install -y pipx git

pipx install pre-commit

pipx ensurepath

source ~/.bashrc

pre-commit run --all-files --show-diff-on-failure

shell: bash

env:

DEBIAN_FRONTEND: noninteractive

Contributing.md

greptile-apps · 2026-01-28T01:01:16Z

.github/workflows/ros-tests.yml


+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git


uncrustify dependency missing. Pre-commit config uses uncrustify (line 46 of .pre-commit-config.yaml) but it's not installed here. Contributing.md also mentions it's required.

Suggested change

apt update && apt install -y pipx git

apt update && apt install -y pipx git uncrustify

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-28T01:09:18Z

.github/workflows/ros-tests.yml


+    - name: Run pre-commit hooks (linting, sign-off check, copyright check, etc.)
+      run: |
+        apt update && apt install -y pipx git


uncrustify is required by the pre-commit config (line 46 of .pre-commit-config.yaml) but isn't installed here. Add it to the apt install command.

Suggested change

apt update && apt install -y pipx git

apt update && apt install -y pipx git uncrustify

Contributing.md

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

Contributing.md

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps

_{6 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

sgillen · 2026-01-28T17:45:37Z

Looks good, thanks! Can you please update the Contributing.md to account for the fact that signing is done automatically, and include the apt install and pipx commands there as well.

bmchalenv · 2026-01-28T17:48:43Z

Looks good, thanks! Can you please update the Contributing.md to account for the fact that signing is done automatically, and include the apt install and pipx commands there as well.

That's already done here. Also signing is not done automatically it just errors if you haven't signed. Usual philosophy with sign-offs is you have to actively acknowledge that it is your own work so I kept that same idea.

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

greenwave_monitor_interfaces/package.xml Show resolved Hide resolved

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

greenwave_monitor_interfaces/package.xml Show resolved Hide resolved

greptile-apps bot reviewed Jan 14, 2026

View reviewed changes

bmchalenv added 7 commits January 27, 2026 13:52

Add pre-commit for linting locally

300f848

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Beginning of linting journey

5ba1541

Signed-off-by: Blake McHale <bmchale@nvidia.com>

More config fixes

dfdf823

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Match flake8 to ament_flake8

624776f

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Run lint again

e42b1d6

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Only support python 3.10 features

1fa80bf

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Only support python 3.10 features

900a432

Signed-off-by: Blake McHale <bmchale@nvidia.com>

bmchalenv force-pushed the pre-commit-init branch from 8b43cc2 to ac97d27 Compare January 27, 2026 23:39

greptile-apps bot reviewed Jan 27, 2026

View reviewed changes

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

bmchalenv added 8 commits January 27, 2026 16:36

Fix line length issue

5c9e1cc

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Reset to main

5898ef0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Fix lints

0489236

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Update Contributing.md to reference installation steps

1ff3421

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Add lint tests as first step since it should fail fast

eb66360

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Tests lint in CI

da401d0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Check sign off pre-commit

b09b7b9

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Check commit

a016e5c

Signed-off-by: Blake McHale <bmchale@nvidia.com>

bmchalenv force-pushed the pre-commit-init branch from 993b9ef to a016e5c Compare January 28, 2026 00:52

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

bmchalenv added 2 commits January 27, 2026 16:57

Update md

d676849

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Use proper working directory

e819d07

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

2f4721d

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

c85bec4

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

bmchalenv added 5 commits January 27, 2026 17:11

Test CI

fb8bd6c

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Test CI

0b771b3

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Test CI

444730b

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Test CI

1cfb152

Signed-off-by: Blake McHale <bmchale@nvidia.com>

Test CI

37f15a0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

38425d7

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

2fac9da

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

1d01220

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

8714eb0

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

387403b

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Test CI

fd0a0aa

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Don't fix smoke tests

7c23de9

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Add uninstall for pre-commit command

9841804

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

Contributing.md Show resolved Hide resolved

Remove ros base

f2c75c4

Signed-off-by: Blake McHale <bmchale@nvidia.com>

greptile-apps bot reviewed Jan 28, 2026

View reviewed changes

sgillen merged commit 022fedf into NVIDIA-ISAAC-ROS:main Jan 28, 2026
17 checks passed

	apt update && apt install -y pipx git
	apt update && apt install -y pipx git uncrustify

Add pre-commit for better linting #25

Add pre-commit for better linting #25

Uh oh!

Conversation

bmchalenv commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

bmchalenv commented Jan 14, 2026 •

edited

Loading

greptile-apps bot commented Jan 14, 2026 •

edited

Loading

bmchalenv commented Jan 28, 2026 •

edited

Loading